fix: banner for usage alert - #191
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
WalkthroughThe Navbar component's JSX return is wrapped in a React fragment, adding a new fixed amber notification banner that displays a message about a user-growth spike and Opus being temporarily down, with guidance to use frontier open models via Concentrate AI. ChangesNavbar Banner Addition
Estimated code review effort: 1 (Trivial) | ~3 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint install failed. For unrecoverable errors, disable the tool in CodeRabbit configuration. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/web/components/homepage/navbar.tsx`:
- Around line 317-327: The homepage notification banner in navbar.tsx needs an
accessibility role so screen readers announce it; update the banner container in
the navbar component to include role="status" (or role="alert" if you want a
more urgent announcement) on the fixed top banner wrapper so assistive tech
picks up the time-sensitive message.
- Around line 317-327: The amber announcement banner in navbar.tsx is hardcoded
to a fixed top offset, so it stays misaligned when BetaCountdownBanner is hidden
and leaves a gap. Update the banner container in the navbar component to derive
its top position from bannerVisible, matching the existing mobile menu overlay
behavior, and use cn() from `@/lib/utils` to conditionally apply the correct top
classes. Keep the banner aligned directly below the header in both states.
- Around line 317-327: The temporary outage banner in the navbar needs a dismiss
control so it doesn’t stay visible forever. Update the banner markup in
navbar.tsx to add a close button and use local state in the navbar component to
hide the banner after dismissal; if the banner is meant to be controlled
centrally, wire it to a server-driven flag instead. Make sure the change is
scoped around the existing fixed banner block and its rendering condition so the
notification can be removed cleanly by the user.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 2145b91d-865c-4b78-979e-9b5ff7d25a36
📒 Files selected for processing (1)
apps/web/components/homepage/navbar.tsx
| <div className="fixed top-[114px] left-0 right-0 z-50 flex items-center justify-center px-4"> | ||
| <div className="flex items-center gap-2 px-4 py-2.5 rounded-full bg-amber-500/10 border border-amber-500/20 text-amber-400 text-[13px] font-mono animate-in fade-in slide-in-from-top-2 duration-500"> | ||
| <span className="w-1.5 h-1.5 rounded-full bg-amber-400 animate-pulse shrink-0" /> | ||
| <span> | ||
| Unexpected spike in user growth & Opus usage — Opus is temporarily down. | ||
| We're working on it. Use{" "} | ||
| <span className="text-amber-300 font-semibold">frontier open models</span> through Concentrate AI in the | ||
| meantime. | ||
| </span> | ||
| </div> | ||
| </div> |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Add role="status" to the banner for screen reader accessibility.
The banner is a time-sensitive notification that should be announced to assistive technology users. Adding role="status" (or role="alert" for more urgent announcements) ensures screen readers pick it up.
♿ Proposed accessibility fix
- <div className="fixed top-[114px] left-0 right-0 z-50 flex items-center justify-center px-4">
+ <div role="status" className="fixed top-[114px] left-0 right-0 z-50 flex items-center justify-center px-4">📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <div className="fixed top-[114px] left-0 right-0 z-50 flex items-center justify-center px-4"> | |
| <div className="flex items-center gap-2 px-4 py-2.5 rounded-full bg-amber-500/10 border border-amber-500/20 text-amber-400 text-[13px] font-mono animate-in fade-in slide-in-from-top-2 duration-500"> | |
| <span className="w-1.5 h-1.5 rounded-full bg-amber-400 animate-pulse shrink-0" /> | |
| <span> | |
| Unexpected spike in user growth & Opus usage — Opus is temporarily down. | |
| We're working on it. Use{" "} | |
| <span className="text-amber-300 font-semibold">frontier open models</span> through Concentrate AI in the | |
| meantime. | |
| </span> | |
| </div> | |
| </div> | |
| <div role="status" className="fixed top-[114px] left-0 right-0 z-50 flex items-center justify-center px-4"> | |
| <div className="flex items-center gap-2 px-4 py-2.5 rounded-full bg-amber-500/10 border border-amber-500/20 text-amber-400 text-[13px] font-mono animate-in fade-in slide-in-from-top-2 duration-500"> | |
| <span className="w-1.5 h-1.5 rounded-full bg-amber-400 animate-pulse shrink-0" /> | |
| <span> | |
| Unexpected spike in user growth & Opus usage — Opus is temporarily down. | |
| We're working on it. Use{" "} | |
| <span className="text-amber-300 font-semibold">frontier open models</span> through Concentrate AI in the | |
| meantime. | |
| </span> | |
| </div> | |
| </div> |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/web/components/homepage/navbar.tsx` around lines 317 - 327, The homepage
notification banner in navbar.tsx needs an accessibility role so screen readers
announce it; update the banner container in the navbar component to include
role="status" (or role="alert" if you want a more urgent announcement) on the
fixed top banner wrapper so assistive tech picks up the time-sensitive message.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Banner position doesn't adapt to bannerVisible state — causes a 44px gap when the countdown banner is hidden.
The amber banner is hardcoded to top-[114px], but the existing mobile menu overlay (line 255) already adjusts between top-[114px] sm:top-[110px] (when bannerVisible is true) and top-[70px] (when false). When BetaCountdownBanner is not visible, the header is only 70px tall, leaving a visible gap between the header and the amber banner.
Use bannerVisible to dynamically set the banner's top offset, and use cn() from @/lib/utils for the conditional classes as per coding guidelines.
🐛 Proposed fix for dynamic banner positioning
- <div className="fixed top-[114px] left-0 right-0 z-50 flex items-center justify-center px-4">
+ <div className={cn(
+ "fixed left-0 right-0 z-50 flex items-center justify-center px-4 transition-all duration-300",
+ bannerVisible ? "top-[114px] sm:top-[110px]" : "top-[70px]"
+ )}>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <div className="fixed top-[114px] left-0 right-0 z-50 flex items-center justify-center px-4"> | |
| <div className="flex items-center gap-2 px-4 py-2.5 rounded-full bg-amber-500/10 border border-amber-500/20 text-amber-400 text-[13px] font-mono animate-in fade-in slide-in-from-top-2 duration-500"> | |
| <span className="w-1.5 h-1.5 rounded-full bg-amber-400 animate-pulse shrink-0" /> | |
| <span> | |
| Unexpected spike in user growth & Opus usage — Opus is temporarily down. | |
| We're working on it. Use{" "} | |
| <span className="text-amber-300 font-semibold">frontier open models</span> through Concentrate AI in the | |
| meantime. | |
| </span> | |
| </div> | |
| </div> | |
| <div className={cn( | |
| "fixed left-0 right-0 z-50 flex items-center justify-center px-4 transition-all duration-300", | |
| bannerVisible ? "top-[114px] sm:top-[110px]" : "top-[70px]" | |
| )}> | |
| <div className="flex items-center gap-2 px-4 py-2.5 rounded-full bg-amber-500/10 border border-amber-500/20 text-amber-400 text-[13px] font-mono animate-in fade-in slide-in-from-top-2 duration-500"> | |
| <span className="w-1.5 h-1.5 rounded-full bg-amber-400 animate-pulse shrink-0" /> | |
| <span> | |
| Unexpected spike in user growth & Opus usage — Opus is temporarily down. | |
| We're working on it. Use{" "} | |
| <span className="text-amber-300 font-semibold">frontier open models</span> through Concentrate AI in the | |
| meantime. | |
| </span> | |
| </div> | |
| </div> |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/web/components/homepage/navbar.tsx` around lines 317 - 327, The amber
announcement banner in navbar.tsx is hardcoded to a fixed top offset, so it
stays misaligned when BetaCountdownBanner is hidden and leaves a gap. Update the
banner container in the navbar component to derive its top position from
bannerVisible, matching the existing mobile menu overlay behavior, and use cn()
from `@/lib/utils` to conditionally apply the correct top classes. Keep the banner
aligned directly below the header in both states.
Source: Coding guidelines
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Add a dismiss mechanism for the temporary outage banner.
The banner is a temporary notification about Opus being down, but there's no way for users to dismiss it. This forces the banner to persist on screen indefinitely, degrading the user experience. Consider adding a close button with local state (or a server-driven flag) to allow dismissal.
♻️ Suggested dismiss button implementation
+ const [alertDismissed, setAlertDismissed] = useState(false)
+
return (
<>
<header className="fixed top-0 left-0 right-0 z-[100]">
...
</header>
- <div className="fixed top-[114px] left-0 right-0 z-50 flex items-center justify-center px-4">
+ {!alertDismissed && (
+ <div className="fixed top-[114px] left-0 right-0 z-50 flex items-center justify-center px-4">
<div className="flex items-center gap-2 px-4 py-2.5 rounded-full bg-amber-500/10 border border-amber-500/20 text-amber-400 text-[13px] font-mono animate-in fade-in slide-in-from-top-2 duration-500">
<span className="w-1.5 h-1.5 rounded-full bg-amber-400 animate-pulse shrink-0" />
<span>
Unexpected spike in user growth & Opus usage — Opus is temporarily down.
We're working on it. Use{" "}
<span className="text-amber-300 font-semibold">frontier open models</span> through Concentrate AI in the
meantime.
</span>
+ <button
+ onClick={() => setAlertDismissed(true)}
+ className="text-amber-400/60 hover:text-amber-400 transition-colors ml-1"
+ aria-label="Dismiss alert"
+ >
+ ✕
+ </button>
</div>
</div>
+ )}
</>🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/web/components/homepage/navbar.tsx` around lines 317 - 327, The
temporary outage banner in the navbar needs a dismiss control so it doesn’t stay
visible forever. Update the banner markup in navbar.tsx to add a close button
and use local state in the navbar component to hide the banner after dismissal;
if the banner is meant to be controlled centrally, wire it to a server-driven
flag instead. Make sure the change is scoped around the existing fixed banner
block and its rendering condition so the notification can be removed cleanly by
the user.
1 participant
Description
Please include a summary of the change and which issue is fixed.
Fixes #(issue)
Type of change
How Has This Been Tested?
Please describe the tests that you ran to verify your changes.
bun testpassesbun run typecheckpassesbun run lintpasses (if applicable)Checklist:
Summary by CodeRabbit